-
Notifications
You must be signed in to change notification settings - Fork 32
CORS Configuration for FHIR API Services #107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@vishwab1 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 23 seconds before requesting another review. β How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. π¦ How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. π Files selected for processing (3)
""" WalkthroughThis change centralizes Cross-Origin Resource Sharing (CORS) configuration by introducing a global CORS configuration class, updating environment properties, and removing all Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Filter (JwtUserIdValidationFilter)
participant SpringMVC (CorsConfig)
participant Controller
Browser->>Filter: Sends HTTP request (with Origin header)
Filter->>Filter: Checks Origin against allowed origins
alt Origin allowed
Filter->>Browser: Sets CORS headers
end
alt OPTIONS request
Filter->>Browser: Responds 200 OK, skips JWT validation
else
Filter->>SpringMVC: Passes request for routing
SpringMVC->>Controller: Handles endpoint logic
Controller-->>Browser: Returns response
end
Possibly related PRs
Suggested reviewers
Poem
β¨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. πͺ§ TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
π Outside diff range comments (4)
src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIDWithUID.java (1)
41-43:headers = "Authorization"may block OPTIONS pre-flight
@RequestMapping(... headers = "Authorization")rejects requests whose header set does not includeAuthorization.
CORS pre-flight calls (OPTIONS) never include this header, so they will be rejected before Springβs CORS layer sees them, nullifying the new central CORS setup.Consider dropping the
headersconstraint or duplicating the mapping without it:-@RequestMapping(value = "/healthIDWithUID", headers = "Authorization", ... +@RequestMapping(value = "/healthIDWithUID", ...src/main/java/com/wipro/fhir/controller/patientdatahandler/HigherHealthFacilityController.java (1)
37-39: Same pre-flight issue withheaders = "Authorization"All routes under this controller inherit the
headers="Authorization"restriction. OPTIONS requests will be blocked.See suggestion in
CreateHealthIDWithUID.java.src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIDWithMobileOTP.java (1)
43-45: Pre-flight blocked byheaders="Authorization"Same concern: OPTIONS calls will not match this mapping. Remove the header constraint or add an alternative mapping.
src/main/java/com/wipro/fhir/controller/eaushdhi/EAushadhiController.java (1)
49-51:headers="Authorization"again blocks OPTIONSSame issue as other controllers. Consolidate the fix project-wide.
π§Ή Nitpick comments (9)
src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIdRecord.java (1)
5-6: Prefer constructor injection over field injection
Field injection via@Autowiredreduces testability and immutability. Switch to constructor-based injection forHealthIDService:- @Autowired - private HealthIDService healthIDService; + private final HealthIDService healthIDService; + + public CreateHealthIdRecord(HealthIDService healthIDService) { + this.healthIDService = healthIDService; + }src/main/java/com/wipro/fhir/controller/test/Test.java (1)
26-27: Prefer constructor injection over field injection
Field injection via@Autowiredreduces testability and immutability. Switch to constructor-based injection forHttpUtilsandOPConsultRecordBundleImpl:- @Autowired - HttpUtils httpUtils; - @Autowired - private OPConsultRecordBundleImpl oPConsultRecordBundleImpl; + private final HttpUtils httpUtils; + private final OPConsultRecordBundleImpl oPConsultRecordBundleImpl; + + public Test(HttpUtils httpUtils, + OPConsultRecordBundleImpl oPConsultRecordBundleImpl) { + this.httpUtils = httpUtils; + this.oPConsultRecordBundleImpl = oPConsultRecordBundleImpl; + }src/main/java/com/wipro/fhir/controller/generateresource/ResourceRequestGateway.java (1)
26-27: Prefer constructor injection over field injection
Field injection with@Autowiredhinders immutability and testing. Convert to constructor-based injection for all three bundles:- @Autowired - private OPConsultRecordBundle opConsultRecordBundle; - @Autowired - private PrescriptionRecordBundle prescriptionRecordBundle; - @Autowired - private DiagnosticReportRecord diagnosticReportRecord; + private final OPConsultRecordBundle opConsultRecordBundle; + private final PrescriptionRecordBundle prescriptionRecordBundle; + private final DiagnosticReportRecord diagnosticReportRecord; + + public ResourceRequestGateway(OPConsultRecordBundle opConsultRecordBundle, + PrescriptionRecordBundle prescriptionRecordBundle, + DiagnosticReportRecord diagnosticReportRecord) { + this.opConsultRecordBundle = opConsultRecordBundle; + this.prescriptionRecordBundle = prescriptionRecordBundle; + this.diagnosticReportRecord = diagnosticReportRecord; + }src/main/java/com/wipro/fhir/controller/patientdatahandler/PatientDataGatewayController.java (1)
29-29: Prefer constructor injection over field injection
Field injection via@Autowiredreduces testability and immutability. Refactor to constructor-based injection forPatientDataGatewayService:- @Autowired - private PatientDataGatewayService patientDataGatewayService; + private final PatientDataGatewayService patientDataGatewayService; + + public PatientDataGatewayController(PatientDataGatewayService patientDataGatewayService) { + this.patientDataGatewayService = patientDataGatewayService; + }src/main/java/com/wipro/fhir/controller/v3/abha/CreateAbhaV3Controller.java (1)
5-6: Prefer constructor injection over field injection
Field injection with@Autowiredreduces testability and immutability. Switch to constructor-based injection forCreateAbhaV3Service:- @Autowired - private CreateAbhaV3Service createAbhaV3Service; + private final CreateAbhaV3Service createAbhaV3Service; + + public CreateAbhaV3Controller(CreateAbhaV3Service createAbhaV3Service) { + this.createAbhaV3Service = createAbhaV3Service; + }src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIDWithMobileOTP.java (1)
133-134: Incorrect logger invocation β placeholder lost
logger.info("NDHM_FHIR Response:", response);passes two arguments but only a single{}placeholder exists, so the second arg is appended bytoString()and the literal colon stays.
Use the placeholder API:-logger.info("NDHM_FHIR Response:", response); +logger.info("NDHM_FHIR Response: {}", response);src/main/java/com/wipro/fhir/utils/FilterConfig.java (1)
23-24: Give the filter a deterministic name
FilterRegistrationBeanwithout an explicit name is fine, but naming helps when
debug-printing the filter chain or tweaking precedence later.registrationBean.setFilter(filter); +registrationBean.setName("jwtUserIdValidationFilter");src/main/java/com/wipro/fhir/utils/JwtUserIdValidationFilter.java (2)
8-9: Remove now-unused@Componentimport
org.springframework.stereotype.Componentis no longer required after the
class stopped being a Spring bean. Leaving it in will trigger an βunused importβ
warning.-import org.springframework.stereotype.Component;
129-146: Regex is too permissive & recomputed per request
matches()is executed on every call; compiling patterns once would be
cheaper.- Without anchors an origin like
http://evil.com.example.comwill satisfy
the ruleexample.com.- return Arrays.stream(allowedOrigins.split(",")) - .map(String::trim) - .anyMatch(pattern -> { - String regex = pattern - .replace(".", "\\.") - .replace("*", ".*") - .replace("http://localhost:.*", "http://localhost:\\d+"); // special case for wildcard port - - boolean matched = origin.matches(regex); - return matched; - }); + return Arrays.stream(allowedOrigins.split(",")) + .map(String::trim) + .map(p -> p.replace(".", "\\.") + .replace("*", ".*") + .replace("http://localhost:.*", "http://localhost:\\d+")) + .map(r -> "^" + r + "$") // anchor the pattern + .anyMatch(origin::matches);
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (20)
src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/java/com/wipro/fhir/config/CorsConfig.java(1 hunks)src/main/java/com/wipro/fhir/controller/carecontext/CareContextController.java(1 hunks)src/main/java/com/wipro/fhir/controller/eaushdhi/EAushadhiController.java(1 hunks)src/main/java/com/wipro/fhir/controller/facility/FacilityController.java(1 hunks)src/main/java/com/wipro/fhir/controller/generateresource/ResourceRequestGateway.java(1 hunks)src/main/java/com/wipro/fhir/controller/healthCard/GenerateHealthIDCardController.java(1 hunks)src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIDWithBio.java(1 hunks)src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIDWithMobileOTP.java(1 hunks)src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIDWithUID.java(1 hunks)src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIdRecord.java(1 hunks)src/main/java/com/wipro/fhir/controller/healthIDvalidate/HealthIDValidateController.java(1 hunks)src/main/java/com/wipro/fhir/controller/patientdatahandler/HigherHealthFacilityController.java(1 hunks)src/main/java/com/wipro/fhir/controller/patientdatahandler/PatientDataGatewayController.java(1 hunks)src/main/java/com/wipro/fhir/controller/test/Test.java(1 hunks)src/main/java/com/wipro/fhir/controller/v3/abha/CreateAbhaV3Controller.java(1 hunks)src/main/java/com/wipro/fhir/controller/v3/abha/LoginAbhaV3Controller.java(1 hunks)src/main/java/com/wipro/fhir/utils/FilterConfig.java(1 hunks)src/main/java/com/wipro/fhir/utils/JwtUserIdValidationFilter.java(4 hunks)
π§° Additional context used
π§ Learnings (4)
src/main/java/com/wipro/fhir/controller/v3/abha/CreateAbhaV3Controller.java (1)
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#52
File: src/main/java/com/wipro/fhir/controller/v3/abha/LoginAbhaV3Controller.java:18-20
Timestamp: 2024-12-09T16:09:31.513Z
Learning: In the Java Spring Boot project, all controllers, including `LoginAbhaV3Controller.java` at `src/main/java/com/wipro/fhir/controller/v3/abha/`, use the `@CrossOrigin` annotation without parameters to maintain consistency across the codebase.
src/main/java/com/wipro/fhir/controller/eaushdhi/EAushadhiController.java (1)
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#52
File: src/main/java/com/wipro/fhir/controller/v3/abha/LoginAbhaV3Controller.java:18-20
Timestamp: 2024-12-09T16:09:31.513Z
Learning: In the Java Spring Boot project, all controllers, including `LoginAbhaV3Controller.java` at `src/main/java/com/wipro/fhir/controller/v3/abha/`, use the `@CrossOrigin` annotation without parameters to maintain consistency across the codebase.
src/main/java/com/wipro/fhir/controller/v3/abha/LoginAbhaV3Controller.java (1)
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#52
File: src/main/java/com/wipro/fhir/controller/v3/abha/LoginAbhaV3Controller.java:18-20
Timestamp: 2024-12-09T16:09:31.513Z
Learning: In the Java Spring Boot project, all controllers, including `LoginAbhaV3Controller.java` at `src/main/java/com/wipro/fhir/controller/v3/abha/`, use the `@CrossOrigin` annotation without parameters to maintain consistency across the codebase.
src/main/java/com/wipro/fhir/controller/carecontext/CareContextController.java (1)
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#52
File: src/main/java/com/wipro/fhir/controller/v3/abha/LoginAbhaV3Controller.java:18-20
Timestamp: 2024-12-09T16:09:31.513Z
Learning: In the Java Spring Boot project, all controllers, including `LoginAbhaV3Controller.java` at `src/main/java/com/wipro/fhir/controller/v3/abha/`, use the `@CrossOrigin` annotation without parameters to maintain consistency across the codebase.
𧬠Code Graph Analysis (1)
src/main/java/com/wipro/fhir/config/CorsConfig.java (1)
src/main/java/com/wipro/fhir/utils/FilterConfig.java (1)
Configuration(9-28)
β° Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
π Additional comments (15)
src/main/environment/common_example.properties (1)
114-116: ```shell
#!/bin/bashShow usage of allowedOriginPatterns in CORS configuration
rg --color=never -n -C 5 "allowedOriginPatterns" src/main/java/com/wipro/fhir/config/CorsConfig.java
Dump the beginning of CorsConfig.java to inspect property mappings
sed -n '1,200p' src/main/java/com/wipro/fhir/config/CorsConfig.java
</details> <details> <summary>src/main/environment/common_ci.properties (1)</summary> `118-120`: **Ensure `CORS_ALLOWED_ORIGINS` environment variable is set and fallback is handled.** If `CORS_ALLOWED_ORIGINS` is missing or empty in CI/deployment, CORS may be effectively disabled. Consider providing a default value or validating presence at startup to avoid unintended blocking. </details> <details> <summary>src/main/java/com/wipro/fhir/controller/healthCard/GenerateHealthIDCardController.java (1)</summary> `26-27`: **Remove controller-level CORS declarations.** Approved: the `@CrossOrigin` import and annotations are removed in favor of a centralized `CorsConfig`. Ensure your global CORS configuration and JWT filter properly handle preflight OPTIONS requests before controller invocation. </details> <details> <summary>src/main/java/com/wipro/fhir/controller/healthIDvalidate/HealthIDValidateController.java (1)</summary> `26-27`: **Remove controller-level CORS declarations.** Approved: removed unused `@CrossOrigin` import, delegating CORS to the global config. Verify that the centralized config allows the required methods and headers (e.g., `Authorization`) for these endpoints. </details> <details> <summary>src/main/java/com/wipro/fhir/controller/facility/FacilityController.java (1)</summary> `5-6`: **Remove controller-level CORS declarations.** Approved: the `@CrossOrigin` import/annotations have been removed. Confirm that your `CorsConfig` covers `/facility/**` GET and POST with `Authorization` header and credentials as needed. </details> <details> <summary>src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIDWithUID.java (1)</summary> `27-33`: **Removed `@CrossOrigin` β confirm central CORS applies** The per-controller `@CrossOrigin` annotations have vanished, relying entirely on the new global `CorsConfig`. Please double-check (manually or with an integration test) that requests to `/healthIDWithUID/**` from the expected origins still carry the correct CORS headers, especially when the `Authorization` header is present and `allowCredentials=true` is required. </details> <details> <summary>src/main/java/com/wipro/fhir/controller/patientdatahandler/HigherHealthFacilityController.java (1)</summary> `24-30`: **Verify CORS coverage after annotation removal** `@CrossOrigin` disappeared here as well; ensure `/higher/health/facility/**` endpoints remain reachable from SPA front-ends after the switch to the central `CorsConfig`. </details> <details> <summary>src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIDWithMobileOTP.java (1)</summary> `28-34`: **Cross-origin removal: confirm still functional** After deleting `@CrossOrigin`, verify that `/healthID/**` remains accessible from configured origins in browsers. </details> <details> <summary>src/main/java/com/wipro/fhir/controller/eaushdhi/EAushadhiController.java (1)</summary> `30-36`: **Confirm CORS works after annotation cleanup** All endpoints under `/eAushadhi/**` now rely on global CORS. Please verify in browser/network tests. </details> <details> <summary>src/main/java/com/wipro/fhir/config/CorsConfig.java (1)</summary> `17-26`: **OPTIONS pre-flight is allowed, but origin pattern API requires Spring 5.3+** `allowedOriginPatterns` exists only from Spring 5.3. If the project is on an older Spring Boot (<2.4) this will not compile. Confirm the framework version or fall back to `.allowedOrigins(...)`. </details> <details> <summary>src/main/java/com/wipro/fhir/utils/FilterConfig.java (1)</summary> `12-14`: **Add a safe default for `cors.allowed-origins`** If the property is missing in some environment Spring will fail during bean creation. Consider providing a default value to keep the service bootable: ```diff - @Value("${cors.allowed-origins}") + @Value("${cors.allowed-origins:*}") // fallback to β*β when not configuredsrc/main/java/com/wipro/fhir/controller/healthID/CreateHealthIDWithBio.java (1)
6-6: Import section looks clean after removing@CrossOriginNo stray or unused imports remain β good job.
src/main/java/com/wipro/fhir/controller/v3/abha/LoginAbhaV3Controller.java (1)
6-6: Controller compiles cleanly after dropping@CrossOriginThe import purge is correct and leaves no unused entries.
src/main/java/com/wipro/fhir/controller/carecontext/CareContextController.java (1)
27-27: Unused@CrossOriginimport successfully removedNothing further to flag for this diff hunk.
src/main/java/com/wipro/fhir/utils/JwtUserIdValidationFilter.java (1)
54-58: OPTIONS short-circuit skips response commit
response.setStatus(200)is fine, but some proxies require a content length
or a flush. Consider adding:response.getWriter().flush();
|



π Description
JIRA ID: AMM-1246
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
β Type of Change
βΉοΈ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
New Features
Refactor
Chores